Codecov Report

❗ No coverage uploaded for pull request base (main@cce8b4e). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #234   +/-   ##
=======================================
  Coverage        ?   76.08%           
=======================================
  Files           ?       29           
  Lines           ?     2204           
  Branches        ?        0           
=======================================
  Hits            ?     1677           
  Misses          ?      527           
  Partials        ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cce8b4e...3bcbb93. Read the comment docs.

I think we can reapply #192 now that jupyterlab/jupyterlab#11744 is merged?
cc @jtpio

It's not clear whether #192 should be considered as a bug fix or not. If so and assuming get_page_config is not really part of the public API then we could reapply it as a bug fix, now that jupyterlab/jupyterlab#11744 handles both cases.

Ok so it would be good to wait for a feedback of @vidartf before taking a decision.

I added a note to bring that up at the next meeting.

Sorry, I was out for the holidays. Did that change break anything more than the test? get_page_config only gives the config from disk, and not the "extra" settings that the handler adds, so it has a somewhat limited scope already. The issue is ofc that it modifies the type of some returned values. Since the dict -> list conversion is mainly intended to be for the benefit of the frontend, I would argue that it makes sense to only apply this transform right before it sends the data.

#192 can be considered more of an enhancement, but it can be argued as a bug fix in that the values in page_config_data are not applied as you would expect.

Did that change break anything more than the test?

Looks like it affected the disabling and enabling of JupyterLab extensions, based on this comment: jupyterlab/jupyterlab#11744 (comment)

#192 can be considered more of an enhancement, but it can be argued as a bug fix in that the values in page_config_data are not applied as you would expect.

So we could maybe reapply #192? Especially now that jupyterlab/jupyterlab#11744 should handle both cases?

So we could maybe reapply #192?

Probably, but it might not make sense as a patch/minor release if it breaks older versions of lab.